Skip to content

Branch support#1065

Open
keremsahn wants to merge 2 commits into
compiler-research:mainfrom
keremsahn:alloc-detect-branch
Open

Branch support#1065
keremsahn wants to merge 2 commits into
compiler-research:mainfrom
keremsahn:alloc-detect-branch

Conversation

@keremsahn

@keremsahn keremsahn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Algorithm:

Instead of copying whole varMap, each branch owns their own undoMap. What undoMap does is, 1)it keeps the original values in varMap to reverse the changes when if branch ends, 2) It keeps the original value of changed variables, and in varMap there is changed version, and when you compare values in varMap and undoMap you find out the deterministic result. undoMap is set as a struct member and keeps the track of undoMap of current branch, it is also kind of like a letter, which inner branches inform their upper branches about what they done, what would be the values if inner did not work.

@keremsahn
keremsahn force-pushed the alloc-detect-branch branch from c321d6a to 174c055 Compare July 9, 2026 18:35
@keremsahn keremsahn changed the title Alloc detect branch Branch support Jul 9, 2026
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.92754% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.21%. Comparing base (321657d) to head (aeeb541).

Files with missing lines Patch % Lines
lib/CppInterOp/CppInterOp.cpp 94.92% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1065      +/-   ##
==========================================
+ Coverage   87.07%   87.21%   +0.14%     
==========================================
  Files          23       23              
  Lines        6073     6197     +124     
==========================================
+ Hits         5288     5405     +117     
- Misses        785      792       +7     
Files with missing lines Coverage Δ
include/CppInterOp/CppInterOpTypes.h 96.90% <ø> (ø)
lib/CppInterOp/CppInterOp.cpp 89.79% <94.92%> (+0.18%) ⬆️
Files with missing lines Coverage Δ
include/CppInterOp/CppInterOpTypes.h 96.90% <ø> (ø)
lib/CppInterOp/CppInterOp.cpp 89.79% <94.92%> (+0.18%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

return AllocType::NewArr;
return AllocType::New;
}
static std::optional<AllocType>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "Cpp::AllocType" is directly included [misc-include-cleaner]

T));
                             ^

struct AllocationTraverser : RecursiveASTVisitor<AllocationTraverser> {
std::unordered_map<const clang::VarDecl*, std::optional<AllocType>> varMap;
std::unordered_map<const clang::FunctionDecl*, std::optional<AllocType>>&
visitedFuncs;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: member 'visitedFuncs' of type 'std::unordered_map<const clang::FunctionDecl *, std::optional> &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]

Type>>&
              ^

std::optional<AllocType>>& cache)
: visitedFuncs(cache) {}

bool TraverseLambdaExpr(clang::LambdaExpr*) { return true; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'TraverseLambdaExpr' can be made static [readability-convert-member-functions-to-static]

Suggested change
bool TraverseLambdaExpr(clang::LambdaExpr*) { return true; }
he) {}static


bool TraverseDecl(clang::Decl* D) {
if (D && llvm::isa<clang::TagDecl>(D))
return true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]

Suggested change
return true;
l* D) {
cl>(D))isa_and_nonnullm::isa<clang::TagDecl>(D)

if (FD->getBuiltinID() == Builtin::ID::BImalloc)
return AllocType::Malloc;
return AnalyzeAllocType(FD);
std::optional<AllocType> join(std::optional<AllocType> a,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'join' can be made static [readability-convert-member-functions-to-static]

Suggested change
std::optional<AllocType> join(std::optional<AllocType> a,
);static

AllocationTraverser Traverser(visitedFuncs);
for (auto* parm : Fn->parameters())
Traverser.VisitVarDecl(parm);
Traverser.TraverseStmt(const_cast<clang::CompoundStmt*>(CmpStmt));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]

(parm);
                                 ^

@keremsahn
keremsahn marked this pull request as draft July 9, 2026 19:52
@keremsahn
keremsahn force-pushed the alloc-detect-branch branch from 174c055 to bd50671 Compare July 9, 2026 23:02
@keremsahn
keremsahn marked this pull request as ready for review July 9, 2026 23:04
TESTAC(31, New);
TESTAC(32, New);
TESTAC(33, New);
TESTAC(34, Unknown);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return New, not Unknown. nullptr should be ignored as "someother type".

TESTAC(39, Unknown);
TESTAC(40, Unknown);
TESTAC(41, Unknown);
TESTAC(42, Unknown);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be New.

@keremsahn
keremsahn force-pushed the alloc-detect-branch branch 2 times, most recently from 8239aae to 8706189 Compare July 16, 2026 17:32

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp
@keremsahn
keremsahn force-pushed the alloc-detect-branch branch 2 times, most recently from 5f4341b to 461c201 Compare July 17, 2026 08:32
@keremsahn
keremsahn force-pushed the alloc-detect-branch branch 4 times, most recently from cce3b53 to 711337a Compare July 20, 2026 13:19
…apper constructions

  Restrict the pointer-arithmetic Unknown-downgrade in VisitBinaryOperator to
  pointer-typed variables only, so unrelated scalar compound-assignments no
  longer poison tracked results.

  Allow AnalyzeAllocType to also traverse record-typed (class/struct) return
  values, not just raw pointers, so wrapper types like std::optional<T*> get
  analyzed instead of short-circuiting to None.

  In handleExpr:
  - Unwrap unary operator* calls (e.g.  on an optional) to
    recurse into the wrapped value instead of falling back to a call analysis.
  - Detect nullptr/NULL literals directly via CXXNullPtrLiteralExpr/
    GNUNullExpr, since the existing CK_NullToPointer cast walk cannot see
    through a MaterializeTemporaryExpr (e.g. nullptr_t passed to optional's
    converting constructor).
  - Recognize implicit converting-constructor calls (ExprWithCleanups ->
    ImplicitCastExpr(CK_ConstructorConversion) -> CXXConstructExpr) and
    recurse into the constructor's argument, so New/Malloc/etc. tags
    propagate through implicit wrapping (e.g. ).
  - Add isNullOpt() to recognize std::nullopt/cpp::nullopt construction as
    AllocType::Null.

  Known limitation (documented via FIXME + test): optional's copy/move
  constructor is not a converting constructor, so copying an optional loses
  its tracked AllocType and reports None.

  Add FunctionReflection_GetAllocType coverage (func60-func69) for: implicit
  optional-wrapping of new, std::nullopt join behavior, default-constructed
  empty structs, direct/list-initialized plain wrapper structs, operator*
  unwrap (unary vs. binary), direct nullptr returns through optional, and
  pointer arithmetic inside a for-loop increment clause.
@keremsahn
keremsahn force-pushed the alloc-detect-branch branch from 711337a to aeeb541 Compare July 20, 2026 13:20

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

}

// Case: constructor cast
if (auto* CCE = dyn_cast<clang::CXXConstructExpr>(finExpr)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto *CCE' can be declared as 'const auto *CCE' [readability-qualified-auto]

Suggested change
if (auto* CCE = dyn_cast<clang::CXXConstructExpr>(finExpr)) {
or castconst

if (const auto* EWC = dyn_cast<clang::ExprWithCleanups>(stripped))
stripped = EWC->getSubExpr();
// We do not want to analyze explicit conversions
auto* ICE = dyn_cast<clang::ImplicitCastExpr>(stripped);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto *ICE' can be declared as 'const auto *ICE' [readability-qualified-auto]

Suggested change
auto* ICE = dyn_cast<clang::ImplicitCastExpr>(stripped);
ersionsconst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants